-
Notifications
You must be signed in to change notification settings - Fork 278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix test failures on 32-bit platforms #2522
Conversation
|
Codecov Report
@@ Coverage Diff @@
## main #2522 +/- ##
==========================================
- Coverage 64.66% 64.61% -0.05%
==========================================
Files 103 103
Lines 22249 22232 -17
Branches 10859 10858 -1
==========================================
- Hits 14388 14366 -22
- Misses 5620 5627 +7
+ Partials 2241 2239 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
4f21d83
to
61e0c1f
Compare
@neheb: I think this is ready now. Previously, I was trying to detect whether we're running on 32-bit or 64-bit and adjust the expected error message accordingly, but that didn't work on Windows. I couldn't figure it out, but I think maybe we're running a 32-bit python binary on Windows. But I discovered an additional problem, which is that some of our tests build a 32-bit exiv2, but run it on a 64-bit platform, so what would actually need to do is figure out whether the exiv2 binary is 32-bit or 64-bit. Instead, I just made these tests less fussy about the expected error message. |
I'd add the other commit to this PR:
|
I don't know if it is helpful or not, but it is possible to find out the bit-length of the exiv2 app using $ exiv2 --verbose --version --grep bits
exiv2 1.0.0.9
Bits=64 |
|
Thanks @postscript-dev! I was wondering if we could do something like that. |
Which platform is that failing on? I just switched on Windows x86 in this PR and everything is passing. (See commit 2: d29acfe) |
I've created a new PR that uses @postscript-dev's idea: #2527. Closing this one. |
Docker command above. Well, with sanitizers enabled. |
These tests are failing because on 32-bit platforms they trigger an exception in a
Safe::add<size_t>()
, which is obviously easier to trigger on 32-bit than on 64-bit. This causes these files to produce a different error message on 32-bit platforms than on 64-bit. I have updated the tests to be less fussy about the exact contents of the error message.